Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typing, linting and example fix #32

Merged
merged 11 commits into from
Nov 21, 2024
Merged

Conversation

jacopoabramo
Copy link
Contributor

A small PR where I did a little bit of clean-up:

  • use of import numpy.typing as npt to replace type hinting (np.ndarray -> npt.NDArray[Any]) where relevant;
  • use of built-in list instead of typing.List (apparently the latter is deprecated from Python 3.9)
  • some small linting adjustments;
  • adjusted some examples that weren't working

For linting I'm using ruff, which is 100% retro-compatible with black but I did not run the linter, I just found some things that he was complaining about via the VSCode extension. For the type hinting, I had a look at how numpy addresses compatibility with mypy and I found numpy.typing.

At some point it would be nice to update the pyproject.toml to include some rules for ruff and mypy but this is up to you if you wish to use these tools. I myself am having a nice time having my code all tidied up.

In case you're aim is to make ExEngine compatible with versions below python 3.9 I can close it but since version 3.8 reached it's end of life a while ago it makes sense to pin the minimum version to 3.9, but it's your call.

@jacopoabramo
Copy link
Contributor Author

I forgot to bump the version, shall I make an extra commit?

@jacopoabramo
Copy link
Contributor Author

@henrypinkard pinging you again; I would like to do some further improvements to the source code by adding some more preliminary interfaces in a new PR and I would like to close this one if you approve it

@henrypinkard henrypinkard merged commit 07067b7 into micro-manager:main Nov 21, 2024
5 checks passed
@henrypinkard
Copy link
Member

Sorry for the delay -- looks good!

Yes I think it makes sense to make 3.9 a minimum version

@henrypinkard
Copy link
Member

Also, @jacopoabramo, @IvoVellekoop has several ideas for improving ExEngine that I believe he may be working on right now which will simplify and improve ExEngine. I think its fantastic that you also have some ideas for improvements -- we should coordinate so that we're all on the same page and effort doesn't get duplicated.

@jacopoabramo
Copy link
Contributor Author

@henrypinkard great to hear and yes, please let's synchronize, next week or the one after. I will be on vacation from the 6th of December to the 8th otherwise I'll be available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants